[Supplier] Implement unbonding period #666
Conversation
red-0ne
added
supplier
WalkthroughThe recent updates to the supplier module enhance the unstaking process, introducing checks for prior unstaking actions and the ability to cancel unbonding when a supplier is restaked. A new Changes
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review due to trivial changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range and nitpick comments (7)
x/supplier/keeper/keeper.go (1)
24-25: Add documentation for new fields.Consider adding comments to document the purpose of the
bankKeeperandsharedKeeperfields for better maintainability and readability.// bankKeeper is responsible for handling bank-related operations. bankKeeper types.BankKeeper // sharedKeeper is responsible for handling shared operations across modules. sharedKeeper types.SharedKeeperx/supplier/keeper/msg_update_params_test.go (1)
33-39: Enhance test descriptions.The test descriptions can be more descriptive to provide better context about the test cases.
- desc: "invalid: send empty params" + desc: "invalid: params should not be empty"testutil/keeper/supplier.go (1)
72-74: Clarify the purpose of moving block height to 1.Explain why the block height is moved to 1 to provide context for future maintainers.
- // Move block height to 1 to get a non zero session end height + // Move block height to 1 to ensure session end height is non-zero for testing purposesx/supplier/keeper/msg_server_stake_supplier.go (1)
52-53: Add a comment explaining the reset of UnstakeCommitSessionEndHeight.Adding a comment will help future maintainers understand why this reset is necessary.
- supplier.UnstakeCommitSessionEndHeight = 0 + // Reset the UnstakeCommitSessionEndHeight as the supplier is staking again. + supplier.UnstakeCommitSessionEndHeight = 0x/supplier/keeper/msg_server_unstake_supplier_test.go (2)
Line range hint
28-63:
Ensure proper block height increment for unbonding period.The test correctly verifies the supplier unbonding process, but it would be clearer to explicitly increment the block height by the unbonding period instead of setting it directly.
- sdkCtx = sdkCtx.WithBlockHeight(unbondingHeight) + sdkCtx = sdkCtx.WithBlockHeight(sdkCtx.BlockHeight() + int64(k.GetParams(ctx).SupplierUnbondingPeriodBlocks))
67-118: Ensure proper block height increment for unbonding period.The test correctly verifies the cancellation of the unbonding period upon restaking. It would be clearer to explicitly increment the block height by the unbonding period instead of setting it directly.
- sdkCtx = sdkCtx.WithBlockHeight(unbondingHeight) + sdkCtx = sdkCtx.WithBlockHeight(sdkCtx.BlockHeight() + int64(k.GetParams(ctx).SupplierUnbondingPeriodBlocks))x/supplier/types/genesis_test.go (1)
Line range hint
62-352:
Ensure test descriptions are clear and consistent.The test cases for the genesis state validation are comprehensive. However, the description for the "invalid - zero supplier unbonding period blocks" test case could be more descriptive.
- desc: "invalid - zero supplier unbonding period blocks", + desc: "invalid - supplier unbonding period blocks set to zero",
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
x/shared/types/supplier.pb.gois excluded by!**/*.pb.gox/supplier/types/params.pb.gois excluded by!**/*.pb.go
Files selected for processing (27)
- api/poktroll/shared/supplier.pulsar.go (16 hunks)
- api/poktroll/supplier/params.pulsar.go (14 hunks)
- e2e/tests/init_test.go (2 hunks)
- e2e/tests/session_steps_test.go (1 hunks)
- e2e/tests/stake_app.feature (2 hunks)
- e2e/tests/stake_gateway.feature (2 hunks)
- e2e/tests/stake_supplier.feature (2 hunks)
- proto/poktroll/shared/supplier.proto (1 hunks)
- proto/poktroll/supplier/params.proto (1 hunks)
- testutil/integration/app.go (4 hunks)
- testutil/keeper/proof.go (1 hunks)
- testutil/keeper/supplier.go (3 hunks)
- testutil/keeper/tokenomics.go (1 hunks)
- x/session/keeper/session_hydrator.go (1 hunks)
- x/supplier/keeper/keeper.go (3 hunks)
- x/supplier/keeper/msg_server_stake_supplier.go (1 hunks)
- x/supplier/keeper/msg_server_unstake_supplier.go (2 hunks)
- x/supplier/keeper/msg_server_unstake_supplier_test.go (4 hunks)
- x/supplier/keeper/msg_update_params_test.go (1 hunks)
- x/supplier/keeper/unbond_suppliers.go (1 hunks)
- x/supplier/module/abci.go (1 hunks)
- x/supplier/module/module.go (6 hunks)
- x/supplier/module/simulation.go (3 hunks)
- x/supplier/types/errors.go (1 hunks)
- x/supplier/types/expected_keepers.go (2 hunks)
- x/supplier/types/genesis_test.go (14 hunks)
- x/supplier/types/params.go (3 hunks)
Additional comments not posted (67)
x/supplier/module/abci.go (1)
1-16: LGTM!The
EndBlockerfunction correctly integrates the unbonding logic by callingEndBlockerUnbondSupplierfrom the keeper package. The changes are approved.proto/poktroll/supplier/params.proto (1)
13-16: LGTM!The
supplier_unbonding_period_blocksfield is correctly added to theParamsmessage with appropriate comments and annotations. The changes are approved.proto/poktroll/shared/supplier.proto (1)
16-16: LGTM!The
unstake_commit_session_end_heightfield is correctly added to theSuppliermessage with appropriate comments. The changes are approved.x/supplier/types/expected_keepers.go (1)
25-29: LGTM!The
SharedKeeperinterface is correctly defined with methods to retrieve shared parameters and session end height information. The changes are approved.x/supplier/keeper/keeper.go (2)
36-36: Ensure proper initialization and usage ofsharedKeeper.The
sharedKeeperis a new dependency added to theNewKeeperfunction. Ensure that this dependency is correctly initialized and used throughout the codebase to avoid potential runtime issues.
48-49: LGTM!The addition of
sharedKeeperto theKeeperstruct and its initialization in the constructor looks good.x/supplier/types/errors.go (1)
19-20: LGTM!The addition of
ErrSupplierUnbondingandErrSupplierParamsInvaliderror types looks good. These errors will help in handling specific conditions related to the supplier unbonding period and invalid parameters.x/supplier/keeper/msg_update_params_test.go (1)
41-48: LGTM!The addition of the test case for
Zero SupplierUnbondingPeriodBlockslooks good. This ensures that the parameter is validated correctly.e2e/tests/stake_gateway.feature (2)
11-11: LGTM!The addition of the step to wait for the
StakeGatewaymessage to be submitted looks good. This ensures that the staking process is completed before proceeding.
23-23: LGTM!The addition of the step to wait for the
UnstakeGatewaymessage to be submitted looks good. This ensures that the unstaking process is completed before proceeding.e2e/tests/stake_app.feature (2)
13-13: Ensure the new step is correctly implemented.The step
And the user should wait for the "application" module "StakeApplication" message to be submittedis added. Verify that the implementation of this step correctly waits for the message.
25-25: Ensure the new step is correctly implemented.The step
And the user should wait for the "application" module "UnstakeApplication" message to be submittedis added. Verify that the implementation of this step correctly waits for the message.e2e/tests/stake_supplier.feature (2)
13-13: Ensure the new step is correctly implemented.The step
And the user should wait for the "supplier" module "StakeSupplier" message to be submittedis added. Verify that the implementation of this step correctly waits for the message.
25-27: Ensure the new steps for unbonding are correctly implemented.The steps for handling the unbonding process are added. Verify that the implementation of these steps correctly handles the unbonding period and transitions.
x/supplier/types/params.go (6)
3-5: Ensure the imports are necessary.The added imports are necessary for the parameter types. No issues here.
7-13: Ensure the new parameter is correctly defined and initialized.The new parameter
SupplierUnbondingPeriodBlocksis defined and initialized with a default value. Ensure that the default value is appropriate and that the TODO comment is addressed before merging.
22-24: Ensure the new parameter is correctly initialized inNewParams.The new parameter
SupplierUnbondingPeriodBlocksis initialized with the default value. No issues here.
34-40: Ensure the new parameter is correctly added toParamSetPairs.The new parameter
SupplierUnbondingPeriodBlocksis added toParamSetPairswith the appropriate validation function. No issues here.
45-51: Ensure the new parameter is correctly validated inValidate.The validation logic for the new parameter
SupplierUnbondingPeriodBlocksis correctly added to theValidatemethod. No issues here.
53-65: Ensure the new validation function is correctly implemented.The validation function for the new parameter
SupplierUnbondingPeriodBlocksis correctly implemented. Ensure that the error messages are clear and that the validation logic is appropriate.x/supplier/keeper/msg_server_unstake_supplier.go (3)
10-10: Ensure the new import is necessary.The import for the
sharedpackage is necessary for accessing shared parameters. No issues here.
41-44: Ensure the check for the unbonding period is correctly implemented.The check for whether the supplier is already in the unbonding period is correctly implemented. No issues here.
47-59: Ensure the logic for setting the unbonding height is correctly implemented.The logic for setting the
UnstakeCommitSessionEndHeightbased on the current height and shared parameters is correctly implemented. No issues here.testutil/keeper/supplier.go (1)
51-59: Ensure shared keeper parameters are set correctly.Verify that the parameters for the shared keeper are set correctly and consider adding error handling for potential issues.
x/supplier/module/simulation.go (2)
70-70: Ensure staking simulation function is implemented correctly.Verify that the
SimulateMsgStakeSupplierfunction is implemented correctly and handles all necessary logic.
81-81: Ensure unstaking simulation function is implemented correctly.Verify that the
SimulateMsgUnstakeSupplierfunction is implemented correctly and handles all necessary logic.x/supplier/keeper/msg_server_stake_supplier.go (1)
52-53: Ensure unstake action is correctly canceled.Verify that the logic for canceling the unstake action when a supplier stakes again is correct and handles all necessary conditions.
x/supplier/keeper/msg_server_unstake_supplier_test.go (3)
Line range hint
131-138:
LGTM!The test correctly verifies the error handling for unstaking a non-existent supplier.
141-164: LGTM!The test correctly verifies the error handling for unstaking outside the unbonding period.
166-186: LGTM!The helper function correctly constructs a stake message for testing purposes.
x/supplier/module/module.go (5)
Line range hint
98-113:
LGTM!The function correctly initializes a new AppModule with the added supplierKeeper.
119-120: LGTM!The function correctly registers the gRPC services for the supplier module.
132-132: LGTM!The function correctly initializes the genesis state for the supplier module.
137-137: LGTM!The function correctly exports the genesis state for the supplier module.
154-156: LGTM!The function correctly handles the end block logic for the supplier module.
testutil/keeper/proof.go (1)
189-189: LGTM!The function correctly initializes the ProofModuleKeepers with the added sharedKeeper.
x/session/keeper/session_hydrator.go (1)
175-179: Exclude suppliers in unbonding periodThis logic correctly excludes suppliers whose
UnstakeCommitSessionEndHeightis set and whose unbonding period has ended before the session end block height. This ensures that only suppliers that are fully available are included in the session.testutil/keeper/tokenomics.go (1)
293-293: AddsharedKeeperto supplier keeperThe
sharedKeeperparameter has been correctly added to the supplier keeper initialization. This ensures that the supplier keeper has access to shared parameters and functionality.e2e/tests/session_steps_test.go (3)
254-281: AddwaitForBlockHeightfunctionThis function correctly waits for a block height to be reached by subscribing to new block events. It ensures that the target height is reached before proceeding, which is essential for handling time-based events in tests.
Line range hint
445-457:
Check if supplier is unbondingThis function correctly checks if a supplier is in the unbonding period by verifying that
UnstakeCommitSessionEndHeightis greater than zero. This ensures that the supplier's unbonding status is accurately determined.
Line range hint
459-465:
Wait for unbonding period to finishThis function correctly waits for the unbonding period to finish by waiting for the block height to reach the supplier's unbonding height. This ensures that the tests account for the unbonding period before proceeding.
e2e/tests/init_test.go (2)
591-609: Retrieve supplier informationThis function correctly retrieves supplier information by querying the supplier module and unmarshalling the response. This ensures that the tests have access to accurate supplier data.
611-629: Calculate supplier unbonding heightThis function correctly calculates the supplier's unbonding height by adding the unbonding period blocks to the
UnstakeCommitSessionEndHeight. This ensures that the tests can accurately determine when the unbonding period will end.api/poktroll/supplier/params.pulsar.go (7)
18-19: Field Descriptor Added:fd_Params_supplier_unbonding_period_blocksThe field descriptor for
supplier_unbonding_period_blocksis correctly added to theParamsstruct.
25-25: Field Descriptor Initialization:fd_Params_supplier_unbonding_period_blocksThe field descriptor for
supplier_unbonding_period_blocksis correctly initialized in theinitfunction.
93-98: Field Added toRangeMethod:supplier_unbonding_period_blocksThe
supplier_unbonding_period_blocksfield is correctly added to theRangemethod.
114-115: Field Added toHasMethod:supplier_unbonding_period_blocksThe
supplier_unbonding_period_blocksfield is correctly added to theHasmethod.
132-133: Field Added toClearMethod:supplier_unbonding_period_blocksThe
supplier_unbonding_period_blocksfield is correctly added to theClearmethod.
150-152: Field Added toGetMethod:supplier_unbonding_period_blocksThe
supplier_unbonding_period_blocksfield is correctly added to theGetmethod.
173-174: Field Added toSetMethod:supplier_unbonding_period_blocksThe
supplier_unbonding_period_blocksfield is correctly added to theSetmethod.testutil/integration/app.go (3)
341-341: Parameter Added:sharedKeepertosupplierKeeperThe
sharedKeeperparameter is correctly added to thesupplierKeeper.
473-476: Function Call:NextBlockto Finalize Genesis and SetupThe
NextBlockfunction call is correctly placed to finalize genesis and setup.
711-714: Context Update: Next BlockThe context update to the next block is correctly done.
api/poktroll/shared/supplier.pulsar.go (14)
69-73: Declaration of new field descriptor looks good.The addition of
fd_Supplier_unstake_commit_session_end_heightis consistent with protobuf conventions.
82-82: Initialization of new field descriptor looks good.The initialization of
fd_Supplier_unstake_commit_session_end_heightin theinitfunction is correctly implemented.
168-173: Inclusion of new field inRangemethod looks good.The addition of
UnstakeCommitSessionEndHeightin theRangemethod ensures proper iteration over the field.
195-196: Inclusion of new field inHasmethod looks good.The addition of
UnstakeCommitSessionEndHeightin theHasmethod ensures proper checking for the presence of the field.
219-220: Inclusion of new field inClearmethod looks good.The addition of
UnstakeCommitSessionEndHeightin theClearmethod ensures proper clearing of the field.
249-251: Inclusion of new field inGetmethod looks good.The addition of
UnstakeCommitSessionEndHeightin theGetmethod ensures proper retrieval of the field.
280-281: Inclusion of new field inSetmethod looks good.The addition of
UnstakeCommitSessionEndHeightin theSetmethod ensures proper setting of the field.
315-316: Inclusion of new field inMutablemethod looks good.The addition of
UnstakeCommitSessionEndHeightin theMutablemethod ensures proper handling of the field as non-mutable.
338-339: Inclusion of new field inNewFieldmethod looks good.The addition of
UnstakeCommitSessionEndHeightin theNewFieldmethod ensures proper initialization of the field.
423-425: Inclusion of new field inProtoMethodssize function looks good.The addition of
UnstakeCommitSessionEndHeightin thesizefunction ensures proper size calculation.
455-459: Inclusion of new field inProtoMethodsmarshal function looks good.The addition of
UnstakeCommitSessionEndHeightin themarshalfunction ensures proper marshaling of the field.
648-666: Inclusion of new field inProtoMethodsunmarshal function looks good.The addition of
UnstakeCommitSessionEndHeightin theunmarshalfunction ensures proper unmarshaling of the field.
721-724: Addition of new field toSupplierstruct looks good.The addition of
UnstakeCommitSessionEndHeightto theSupplierstruct is correctly implemented.
768-773: Addition of getter method for new field looks good.The addition of
GetUnstakeCommitSessionEndHeightensures proper access to the new field.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/shared/types/supplier.pb.gois excluded by!**/*.pb.go
Files selected for processing (11)
- api/poktroll/shared/supplier.pulsar.go (16 hunks)
- e2e/tests/init_test.go (2 hunks)
- proto/poktroll/shared/supplier.proto (1 hunks)
- testutil/keeper/supplier.go (3 hunks)
- x/session/keeper/session_hydrator.go (1 hunks)
- x/supplier/keeper/msg_server_stake_supplier.go (1 hunks)
- x/supplier/keeper/msg_server_unstake_supplier.go (2 hunks)
- x/supplier/keeper/msg_server_unstake_supplier_test.go (4 hunks)
- x/supplier/keeper/unbond_suppliers.go (1 hunks)
- x/supplier/types/errors.go (1 hunks)
- x/supplier/types/params.go (3 hunks)
Files skipped from review as they are similar to previous changes (9)
- api/poktroll/shared/supplier.pulsar.go
- e2e/tests/init_test.go
- proto/poktroll/shared/supplier.proto
- testutil/keeper/supplier.go
- x/supplier/keeper/msg_server_stake_supplier.go
- x/supplier/keeper/msg_server_unstake_supplier.go
- x/supplier/keeper/msg_server_unstake_supplier_test.go
- x/supplier/keeper/unbond_suppliers.go
- x/supplier/types/params.go
Additional comments not posted (7)
x/supplier/types/errors.go (2)
19-19: LGTM!The error
ErrSupplierIsUnstakingis clear and consistent with the new unbonding period feature.
20-20: LGTM!The error
ErrSupplierParamsInvalidis clear and consistent with the error handling improvements.x/session/keeper/session_hydrator.go (5)
175-175: LGTM!The check to exclude suppliers that are in an unbonding period is consistent with the new unbonding period feature.
176-177: LGTM!The TODO comment regarding newly staked suppliers joining sessions immediately highlights a potential issue that needs to be addressed in the future.
178-179: LGTM!The check for
UnstakeCommitSessionEndHeightensures that suppliers who are unstaking are not included in the session's suppliers list.
180-180: LGTM!The continuation of the check for
UnstakeCommitSessionEndHeightensures that the logic is complete and consistent.
181-182: LGTM!The blank line improves the readability of the code.
|
The CI will now also run the e2e tests on devnet, which increases the time it takes to complete all CI checks. You may need to run GCP workloads (requires changing the namespace to 666) |
github-actions
bot
added
devnet
push-image
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/supplier/keeper/unbond_suppliers.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/supplier/keeper/unbond_suppliers.go
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- testutil/integration/app.go (4 hunks)
- testutil/keeper/proof.go (1 hunks)
- testutil/keeper/supplier.go (4 hunks)
- testutil/keeper/tokenomics.go (1 hunks)
- x/session/keeper/session_hydrator.go (2 hunks)
- x/supplier/keeper/keeper.go (3 hunks)
- x/supplier/keeper/msg_server_stake_supplier.go (1 hunks)
- x/supplier/module/module.go (7 hunks)
- x/supplier/module/simulation.go (3 hunks)
- x/supplier/types/errors.go (1 hunks)
- x/supplier/types/expected_keepers.go (1 hunks)
Files skipped from review as they are similar to previous changes (11)
- testutil/integration/app.go
- testutil/keeper/proof.go
- testutil/keeper/supplier.go
- testutil/keeper/tokenomics.go
- x/session/keeper/session_hydrator.go
- x/supplier/keeper/keeper.go
- x/supplier/keeper/msg_server_stake_supplier.go
- x/supplier/module/module.go
- x/supplier/module/simulation.go
- x/supplier/types/errors.go
- x/supplier/types/expected_keepers.go
e2e/tests/init_test.go
Outdated
| var resp sharedtypes.QueryParamsResponse | ||
| responseBz := []byte(strings.TrimSpace(res.Stdout)) | ||
| s.cdc.MustUnmarshalJSON(responseBz, &resp) | ||
| windowCloseHeight := shared.GetProofWindowCloseHeight( |
There was a problem hiding this comment.
Shouldn't we get this from the supplier field?
There was a problem hiding this comment.
The supplier has the SessionEndHeight of the block at which the MsgUnstake tx was committed.
I'll try storing the UnbondingHeight to see if its better API.
The reason for storing UnstakeSessionEndHeight and not the windowCloseHeight is that we need the former to know whether the Supplier is still active or not.
Storing the additional windowCloseHeight/UnbondingHeight would be redundant since we can derive it from UnstakeSessionEndHeight.
As @bryanchriswhite suggested, Having a static function to get the UnbondingHeight should be the way to go.
x/session/keeper/session_hydrator.go
Outdated
| // Exclude suppliers that are in an unbonding period. | ||
| // TODO_TECHDEBT: Suppliers that stake mid-session SHOULD NOT be included | ||
| // in the current session's suppliers list and must wait until the next one. | ||
| if s.UnstakeCommitSessionEndHeight != suppliertypes.SupplierNotUnstaking && |
There was a problem hiding this comment.
can we create a func (s *Supplier) isUnbonding() bool function as a helper?
There was a problem hiding this comment.
It would have to take a height argument.
func (s *Supplier) isUnbonding(queryHeight int64) bool
There was a problem hiding this comment.
@bryanchriswhite, I think there's a nuance here that needs to be considered given the value of UnstakeSessionEndHeight:
0: never unstaked!= 0 && < currentSessionEnd: initiated an unstake but still active (serving relays)> currentSessionEnd: is unbonding and no longer active (serving relays)
I believe IsUnbonding should only indicate if the supplier initiated an unstake, and not whether it's sill active or not. This would maximize the reusablility of the method.
Given that, it's signature should be IsUnbonding() bool and should not take the height as argument.
We could still have a IsActive(queryHeight) bool that Indicates whether it should be included in sessions or not.
| // Removing it right away could have undesired effects on the network | ||
| // (e.g. a session with less than the minimum or 0 number of suppliers, | ||
| // off-chain actors that need to listen to session supplier's change mid-session, etc). | ||
| supplier.UnstakeCommitSessionEndHeight = uint64(shared.GetSessionEndHeight(&sharedParams, currentHeight)) |
There was a problem hiding this comment.
This should be based on a governance param based on the original ticket.
There was a problem hiding this comment.
We are not storing when the unbonding should be effective, but rather when the Supplier becomes inactive.
The unbonding height is derived from this one at the EndBlocker level.
x/session/keeper/session_hydrator.go
Outdated
| // Exclude suppliers that are in an unbonding period. | ||
| // TODO_TECHDEBT: Suppliers that stake mid-session SHOULD NOT be included | ||
| // in the current session's suppliers list and must wait until the next one. | ||
| if s.UnstakeCommitSessionEndHeight != suppliertypes.SupplierNotUnstaking && |
There was a problem hiding this comment.
It would have to take a height argument.
func (s *Supplier) isUnbonding(queryHeight int64) bool
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
x/shared/types/supplier.go (1)
11-13: Add comments explaining the logic.To improve readability, add comments explaining the logic of the function.
func (s *Supplier) IsActive(queryHeight int64) bool { + // A supplier is active if it is not unbonding or if the query height is within the unbonding period return !s.IsUnbonding() || uint64(queryHeight) <= s.UnstakeSessionEndHeight }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
x/shared/types/supplier.pb.gois excluded by!**/*.pb.go
Files selected for processing (12)
- api/poktroll/shared/supplier.pulsar.go (16 hunks)
- e2e/tests/init_test.go (3 hunks)
- proto/poktroll/shared/supplier.proto (1 hunks)
- testutil/keeper/supplier.go (4 hunks)
- x/session/keeper/session_hydrator.go (1 hunks)
- x/shared/supplier.go (1 hunks)
- x/shared/types/supplier.go (1 hunks)
- x/supplier/keeper/msg_server_stake_supplier.go (1 hunks)
- x/supplier/keeper/msg_server_unstake_supplier.go (2 hunks)
- x/supplier/keeper/msg_server_unstake_supplier_test.go (3 hunks)
- x/supplier/keeper/unbond_suppliers.go (1 hunks)
- x/supplier/module/abci.go (1 hunks)
Files skipped from review as they are similar to previous changes (10)
- api/poktroll/shared/supplier.pulsar.go
- e2e/tests/init_test.go
- proto/poktroll/shared/supplier.proto
- testutil/keeper/supplier.go
- x/session/keeper/session_hydrator.go
- x/supplier/keeper/msg_server_stake_supplier.go
- x/supplier/keeper/msg_server_unstake_supplier.go
- x/supplier/keeper/msg_server_unstake_supplier_test.go
- x/supplier/keeper/unbond_suppliers.go
- x/supplier/module/abci.go
Additional comments not posted (2)
x/shared/types/supplier.go (2)
3-5: LGTM!The constant
SupplierNotUnstakingis correctly defined.
7-9: LGTM!The function
IsUnbondingis correctly implemented.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- x/shared/types/supplier.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- x/shared/types/supplier.go
Co-authored-by: Daniel Olshansky <[email protected]>
Summary
This PR adds an unbonding period when a
Suppliersubmits an unstaking transaction.unbondingHeightto theSupplierproto.unbondingHeightwhen a supplier re-stakes during that period.unbondingHeightfor a given height.Suppliers that are already in an unbonding period.EndBlockerroutine to effectively unbondSuppliers that reached their unbonding height.Suppliers normally functioning during the unbonding period.Issue
Suppliers that submit anunstaketransaction have to wait for a certain time before getting their staked funds back.Type of change
Select one or more:
Testing
Documentation changes (only if making doc changes)
make docusaurus_start; only needed if you make doc changesLocal Testing (only if making code changes)
make go_develop_and_testmake test_e2ePR Testing (only if making code changes)
devnet-test-e2elabel to the PR.make trigger_ciif you want to re-trigger tests without any code changesSanity Checklist
Summary by CodeRabbit
New Features
EndBlockerUnbondSuppliersfunction to manage supplier unbonding at session end.Bug Fixes
Tests
Chores